Reviewing files that changed from the base of the PR and between 36999a4 and c434b67.

• abci/Cargo.toml (1 hunks)
• abci/src/lib.rs (1 hunks)
• abci/src/server/generic.rs (1 hunks)
• proto-compiler/Cargo.toml (0 hunks)
• proto-compiler/src/constants.rs (2 hunks)
• proto-compiler/src/lib.rs (1 hunks)
• proto/Cargo.toml (2 hunks)
• proto/build.rs (1 hunks)
• proto/src/.gitignore (1 hunks)
• proto/src/lib.rs (2 hunks)
• proto/src/time.rs (1 hunks)

• proto-compiler/Cargo.toml

• proto/src/.gitignore

• GitHub Check: test-all

proto/build.rs (2)

16-19: LGTM! Clear documentation of build behavior.

The added comment effectively explains that building both server and client components is safe, even if they're not used in lib.rs.

---

20-25: Verify feature flag dependencies.

The conditional compilation directives correctly separate server and client builds. However, we should verify that all necessary dependencies are properly configured in Cargo.toml.

✅ Verification successful

Feature flag dependencies are correctly configured ✅

The conditional compilation in build.rs is properly supported by the feature flag hierarchy in Cargo.toml:

• Server feature includes client capabilities and transport
• Client feature provides minimal gRPC client support
• Tonic dependency is properly gated with appropriate features for each use case

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify feature flag configurations in Cargo.toml files
# Check for proper feature definitions and dependencies

# Search for feature definitions
echo "Checking feature definitions in Cargo.toml files:"
rg -A 5 '^\[features\]' 

# Search for feature usage in dependencies
echo "Checking feature usage in dependencies:"
rg -A 5 'features\s*=\s*\[' 

Length of output: 4592

---

Script:

#!/bin/bash
# Get the complete features section from proto/Cargo.toml
echo "Features section from proto/Cargo.toml:"
rg -A 20 '^\[features\]' proto/Cargo.toml

# Check if tonic is properly gated by features
echo -e "\nTonic dependency configuration:"
rg 'tonic.*=' proto/Cargo.toml

Length of output: 1283

abci/src/lib.rs (2)

42-44: LGTM! Improved error handling structure.

The removal of #[from] attributes in favor of explicit type specifications enhances clarity and control over error conversions.

---

53-65: Well-structured manual implementations for no_std compatibility.

The manual implementations of From traits for EncodeError and DecodeError are correctly implemented and documented. This change improves compatibility with no_std environments while maintaining clean error conversion paths.

proto/src/time.rs (2)

9-11: LGTM! Improved documentation clarity.

The added note about resolution differences between protobuf timestamp and milliseconds is helpful for developers.

---

3-5: Verify format module usage across features.

The conditional compilation ensures the format module is only included when necessary. However, we should verify that this doesn't break any existing functionality.

✅ Verification successful

Conditional compilation of format module is correctly implemented

The format module is properly isolated and only imported in the nostd context. No conflicts or breaking changes were found in server/client feature paths. The timestamp formatting functionality remains intact through specialized handling.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify format module usage
# Check if format module is used in server/client feature contexts

# Search for format module usage
echo "Checking format module usage:"
rg -p "use.*format" 

# Search for potential timestamp formatting calls
echo "Checking timestamp formatting calls:"
rg -p "format!.*time" 

Length of output: 593

---

Script:

#!/bin/bash
# Check format module contents and direct references
echo "Checking format module contents:"
rg -p "mod format|pub mod format"

echo "Checking direct format module references:"
rg -p "format::" --type rust

# Check feature-gated code blocks
echo "Checking feature-gated blocks:"
ast-grep --pattern 'cfg(feature = $_)'

Length of output: 344

abci/src/server/generic.rs (1)

Line range hint 99-106: LGTM! Simplified type constraints.

The removal of the unnecessary lifetime parameter 'a from the Server implementation for GenericServer is a good simplification. The implementation maintains all necessary trait bounds (Send + Sync + 'static) while reducing complexity.

proto-compiler/src/lib.rs (2)

127-138: LGTM! Clean separation of server configuration.

The GrpcServer configuration properly enables all required components (client, server, transport) while maintaining feature gates.

---

139-149: LGTM! Well-optimized client configuration for WASM.

The GrpcClient configuration correctly disables server and transport components while keeping client functionality, which aligns with WASM target requirements.

proto/src/lib.rs (3)

6-6: LGTM! Documentation style fix.

Allowing clippy::doc_lazy_continuation improves documentation readability without affecting functionality.

---

37-39: LGTM! Clean module organization.

The conditional compilation logic properly separates the modules based on features, with clear boundaries between server, client, and nostd implementations.

Also applies to: 42-49

---

51-58: LGTM! Well-structured re-exports.

The re-export logic maintains a clear precedence (server > client > nostd) while ensuring only one implementation is exposed.

proto-compiler/src/constants.rs (2)

16-19: LGTM! Clear enum variants.

The GenerationMode enum variants clearly represent their purposes with good documentation.

---

35-42: LGTM! Descriptive Display implementation.

The Display trait implementation provides clear and consistent string representations of the generation modes.

abci/Cargo.toml (1)

35-35: LGTM! Proper feature dependency.

Adding grpc to server feature ensures all required functionality is available.

proto/Cargo.toml (2)

55-55: LGTM: Optimized tonic dependency configuration

Good optimization! Disabling default features reduces the dependency footprint and aligns with making the crate more WASM-friendly.

---

37-37: Breaking change: Default feature changed to "server"

This change modifies the default behavior from including just gRPC support to including both client and server features. While this aligns with the PR objectives, it could impact existing users.

Let's verify the impact on existing dependencies:

Consider:

1. Adding a migration guide in the README
2. Documenting the new feature flags in the crate documentation

✅ Verification successful

Breaking change impact is minimal, documentation updates recommended

The change to default features appears safe as:

• No dependencies found relying on default features
• The abci package explicitly opts out of defaults with default-features = false

Still recommended:

• Document the feature flag changes in CHANGELOG.md
• Update crate documentation to explain the new feature structure

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for dependencies on tenderdash-proto without explicit feature selection
rg --type toml -A 5 'tenderdash-proto = .*(?!features)'

Length of output: 315

---

Script:

#!/bin/bash
# Find all Cargo.toml files that depend on tenderdash-proto
rg --type toml -l "tenderdash-proto"

# For each found file, show the dependency specification and surrounding context
rg --type toml -A 5 -B 2 'tenderdash-proto = .*'

Length of output: 651

---

Script:

#!/bin/bash
# Extract the tenderdash-proto dependency configuration from proto-compiler
rg --type toml -A 5 -B 2 'tenderdash-proto = .*' proto-compiler/Cargo.toml

Length of output: 76

Reviewing files that changed from the base of the PR and between c434b67 and 82cc923.

• abci/Cargo.toml (1 hunks)
• proto/Cargo.toml (2 hunks)
• proto/src/error.rs (1 hunks)
• proto/src/lib.rs (2 hunks)
• proto/src/prelude.rs (1 hunks)
• proto/src/protobuf.rs (1 hunks)
• proto/src/serializers/from_str.rs (1 hunks)
• proto/src/serializers/part_set_header_total.rs (1 hunks)
• proto/src/serializers/time_duration.rs (1 hunks)
• proto/src/serializers/timestamp.rs (1 hunks)
• proto/tests/unit.rs (1 hunks)

• proto/tests/unit.rs
• proto/src/serializers/timestamp.rs

• abci/Cargo.toml

• GitHub Check: test-all
• GitHub Check: build (alpine)

proto/src/prelude.rs (1)

14-14: LGTM! Verify tonic dependency configuration

The change from grpc to std feature for tonic re-export is semantically correct since tonic requires the standard library. This aligns with the PR objective to deprecate the grpc feature.

Let's verify the tonic dependency configuration:

✅ Verification successful

Verified: Tonic dependency and feature flags are properly configured

The change to #[cfg(feature = "std")] is correct and well-structured:

• The std feature enables tonic through the client feature
• This aligns with deprecating the grpc feature
• The dependency hierarchy ensures proper feature resolution

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if tonic is properly configured in Cargo.toml
rg -l 'tonic.*=.*' | while read -r file; do
  echo "=== $file ==="
  rg 'tonic.*=.*' "$file"
done

Length of output: 408

---

Script:

#!/bin/bash
# Check feature definitions in Cargo.toml files
for file in proto/Cargo.toml proto-compiler/Cargo.toml; do
  echo "=== $file ==="
  rg '^\[features\]' -A 10 "$file"
done

Length of output: 908

---

Script:

#!/bin/bash
# Get complete feature definitions and tonic-related dependencies from proto/Cargo.toml
echo "=== Complete proto/Cargo.toml ==="
cat proto/Cargo.toml

Length of output: 2781

proto/src/serializers/time_duration.rs (1)

2-5: LGTM! Appropriate feature flag for Duration type

The change from grpc to std feature for Duration import is semantically correct, as it properly reflects the choice between std and core libraries.

proto/src/serializers/from_str.rs (1)

4-7: LGTM! Appropriate feature flag for trait imports

The change from grpc to std feature for FromStr and Display trait imports is semantically correct, as it properly reflects the choice between std and core libraries.

proto/Cargo.toml (1)

37-52: LGTM! Clear separation of features for better granularity.

The new feature structure effectively separates client and server capabilities, which is crucial for WASM support. The deprecation notices and feature dependencies are well-organized.

Let's verify the impact of these feature changes on dependent crates:

✅ Verification successful

Feature changes are safe to proceed

The verification shows that:

• Dependencies are already using the new feature flags (server, std)
• No external usage of the deprecated grpc feature was found
• The only grpc feature reference is in the internal proto-compiler crate

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for direct dependencies on the deprecated 'grpc' feature
rg --type rust -A 3 'tenderdash-proto.*features.*=.*["'']grpc'

Length of output: 62

---

Script:

#!/bin/bash
# Find all Cargo.toml files and check for tenderdash-proto dependencies
fd Cargo.toml --type f --exec sh -c 'echo "=== {}:" && cat {} | grep -A 5 "tenderdash-proto"'

# Also search for any direct references to the features in rust files
rg --type rust -e 'tenderdash.proto.*features' -e '#\[cfg\(feature.*grpc'

Length of output: 1886

proto/src/lib.rs (2)

3-6: LGTM! Improved feature flag organization.

The change from grpc to std for no_std configuration is more semantically correct, and the added clippy allowance is appropriate for documentation.

---

37-58: Well-structured conditional compilation for module separation.

The module organization clearly separates client and server functionality while maintaining compatibility through re-exports. This is crucial for WASM support where server functionality is not needed.

Let's verify the module organization:

✅ Verification successful

Module separation is correctly implemented and verified.

The codebase maintains clean separation between server, client, and no_std implementations through feature flags, with no unexpected cross-dependencies. The organization supports WASM compatibility by properly isolating server functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any potential cross-dependencies between client and server modules
rg -A 3 'use (crate::)?(tenderdash_grpc|tenderdash_grpc_client)' --type rust

Length of output: 392

---

Script:

#!/bin/bash
# Check module definitions and their contents
echo "=== Checking module definitions ==="
fd -e rs -x head -n 20 {}

echo -e "\n=== Checking for any tonic/grpc related imports ==="
rg "use tonic|use grpc" --type rust

Length of output: 20579

proto/src/serializers/part_set_header_total.rs (1)

9-11: LGTM! Consistent feature flag usage for imports.

The change from grpc to std feature flag for conditional imports is consistent with the new feature organization.

proto/src/protobuf.rs (1)

6-8: LGTM! Consistent feature flag usage for imports.

The change from grpc to std feature flag for conditional imports aligns with the new feature organization.